Skip to content

test(server): skip the pathspec-magic filename test on Windows - #15651

Open
OhadC wants to merge 2 commits into
pingdotgg:mainfrom
OhadC:skip-pathspec-test-on-windows
Open

test(server): skip the pathspec-magic filename test on Windows#15651
OhadC wants to merge 2 commits into
pingdotgg:mainfrom
OhadC:skip-pathspec-test-on-windows

Conversation

@OhadC

@OhadC OhadC commented Oct 4, 2026

Copy link
Copy Markdown

Problem

GitVcsDriverCore.test.ts › "keeps untracked filenames with pathspec magic in the review" (added in #8086) always fails on Windows. It writes a file named :(exclude)after.ts, and NTFS never allows : in a file name, so the setup step fails with ENOENT before any assertion runs:

PlatformError: NotFound: FileSystem.writeFile (…\git-vcs-driver-test-…\:(exclude)after.ts)

Change

Skip the test on Windows with it.effect.skipIf(HostProcessPlatform.defaultValue() === "win32"), the same guard this file already uses for the newline-in-path test. The diff is mostly re-indentation from wrapping the test.

Scope and approval

A very small, focused fix of an obvious test bug: the file name the test needs can't exist on Windows, so there is no Windows behavior for it to check. The other platforms still run it unchanged.

Verification

On Windows 11, Node 24:

  • Before: vp test run src/vcs/GitVcsDriverCore.test.ts (from apps/server) fails this test with the error above.
  • After: vp test run src/vcs/GitVcsDriverCore.test.ts -t "review diff previews" gives 29 passed and the rest skipped, including this test. No failures.

I couldn't run it on macOS or Linux, but nothing changes there.

Done with Claude Opus 5.5 in Claude Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 4, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at db6a886

Macroscope's review found this PR approvable — This is a single test-file change confined to an ignored path, adjusting only Windows test execution without affecting production behavior, product defaults, or static-analysis settings. Its narrowly scoped test-harness impact makes it low risk.

Notes:

  • No code objects were reviewed. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 02094f31-9b4a-422f-ba59-ce618b39c5b3
📥 Commits

Reviewing files that changed from the base of the PR and between db6a886 and 213fbc3.

📒 Files selected for processing (1)
  • apps/server/src/vcs/GitVcsDriverCore.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

The untracked-pathspec review test now skips on Windows. On other platforms, it checks preview contents and verifies that previewing leaves the Git index unchanged.

Changes

Untracked pathspec test

Layer / File(s) Summary
Platform-aware pathspec assertions
apps/server/src/vcs/GitVcsDriverCore.test.ts		 The test skips on Windows. On other platforms, it checks full and file-scoped previews and confirms that previewing does not change the Git index.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Other

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 213fb

The affected test is skipped on Windows while its checks remain enabled on other platforms. No actionable merge risk remains.

Architecture Summary

Architecture risk: 🔵 Low · up to 213fb

The change affects 1 system.

Changed systems: apps/server

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/server (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/server/src/vcs/GitVcsDriverCore.test.ts: The test for reviewing an untracked file named :(exclude)after.ts is now skipped when the host platform is Windows; on other platforms, it retains the same checks for full and file-scoped previews and verifies the Git index is unchanged.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: skipping the pathspec-magic filename test on Windows.
Description check ✅ Passed The description covers the problem, change, scope and approval rationale, and verification results. It also states which platforms were not tested.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Approvability ✅ Passed The pull request changes only apps/server/src/vcs/GitVcsDriverCore.test.ts. It adds a Windows-only skip to one test that creates a pathspec-magic filename. This is a focused test-only change. It doe…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes
@macroscopeapp macroscopeapp[bot] macroscopeapp[bot] approved these changes

Assignees

No one assigned

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@OhadC